Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow queryset slicing and running a function on the materialised relationship elements #85

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

adammsteele
Copy link

Related to: #67

I just wanted to know whether there was an appetite for this kind of functionality in the core library, I'd be happy to contribute more tests and documentation if you are open to adding it.

I'm not precious about the code, feel free to tear it to shreds, this was a quick implementation to demonstrate the functionality I want rather than just asking for it.

This allows you to slice querysets and iterables, and optionally collapse iterables with one element into just that element.
Add a generic function to be run after the results have been materialised, and actually slice the queryset so the database can limit the results returned.
@adammsteele
Copy link
Author

On second thought, this is really two proposals, one to slice querysets where possible, and another to allow an arbitrary function to be run after materialising relationship lists.

@adammsteele adammsteele changed the title Add slice and collapse arguments to map_or_apply Allow queryset slicing and running a function on the materialised relationship elements Jun 10, 2023
@j4mie
Copy link
Member

j4mie commented Jun 12, 2023

Huge thanks for this! Really appreciate you taking the time to dig in and make a contribution.

In terms of the change itself, I think slicing a queryset and then returning just (say) the first item is something I'd be interested in including in the library. The question is, at what level do we do this? You've taken the approach of baking support deep into the guts of the library. I'm not totally sure this is necessary. I feel like this is something that it should be possible to support at the higher levels (ie pairs) without making too many deep-level changes to (say) qs. My approach to adding features to django-readers is always to start with: could this be done in application code without adding features to readers itself? If so, what features might we consider adding to make it simpler? So start at the top and work down, rather than starting at the bottom and working up.

As an example: I don't think it's necessary to include a separate slice parameter to the relationship functions in qs, because the prepare_related_queryset parameter should already be general-purpose enough to do this:

def apply_slice(slice):
    def prepare(queryset):
        return queryset[slice]

Anyway, let me have a think about this over the next few days and I'll comment with some more thoughts 🙂

Thanks again

@adammsteele
Copy link
Author

Totally appreciate that you don't want to add a larger surface area to the library unless you need to.

I can see now that slicing querysets could be done with a slice pair, or even directly slicing the related_queryset that you would pass to the reverse_relationship pair, I think I was getting tripped up by the fact that slicing has to be done as the last step of queryset preparation, so potentially the solutions you have offered aren't as easily composable as something built in to the library.

I'm looking forward to hearing your thoughts, thanks for taking the time to consider this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants